Skip to content

Guard benchmark summary against zero-time compare ratios#219

Closed
Copilot wants to merge 3 commits into
developfrom
copilot/fix-code-for-review-comments
Closed

Guard benchmark summary against zero-time compare ratios#219
Copilot wants to merge 3 commits into
developfrom
copilot/fix-code-for-review-comments

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This addresses the remaining benchmark.sh review-thread issue in PR #217. The compare summary could divide by zero when the branch average rounded to 0, breaking output for very small runs.

  • Summary ratio handling

    • Guard cmp/branch computation when branch_avg == 0
    • Emit stable summary values instead of dividing by zero:
      • inf when compare is non-zero and branch rounds to zero
      • n/a when both sides round to zero
  • Summary labeling

    • Make the zero-rounded case explicit in the note column with branch ~0ms
  • Scope

    • Limit changes to the summary awk block in benchmark.sh
    • Leave normal non-zero timing behavior unchanged
# before
cmp_branch = u2 / u1

# after
if (u1 > 0) {
  cmp_branch = u2 / u1
  sp = sprintf("%10.2fx", cmp_branch)
} else if (u2 > 0) {
  cmp_branch = "inf"
  sp = sprintf("%10s", "inf")
} else {
  cmp_branch = ""
  sp = sprintf("%10s", "n/a")
}

Copilot AI changed the title [WIP] Fix code for all comments in review thread Guard benchmark summary against zero-time compare ratios Jun 29, 2026
Copilot AI requested a review from tameware June 29, 2026 04:06

@tameware tameware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip this change. If need be, we'll update timer.cpp instead.

@tameware tameware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants